UUID partition change #4914
UUID partition change #4914
Conversation
|
|
||
| value, err = uuids.getUint64() | ||
| require.NoError(t, err) | ||
| require.Equal(t, value, maxUint56Minus2+1) | ||
|
|
||
| value, err = uuids.GenerateUUID() | ||
| require.NoError(t, err) | ||
| require.Equal(t, value, uint64(0xdefffffffffffffe)) | ||
| require.Equal(t, value, uint64(0xffffdefffffffffe)) | ||
|
|
There was a problem hiding this comment.
I'm not sure about the implementation.
We know that the UUID generator generates UUID for the same partition, and the partition is determined by the block ID and transaction index.
In other words, given a UUID Generator, if we keep calling GenerateUUID, the returned value always the same byte at the partition byte position, which is the 3rd byte.
However, L201 takes the stored uuid value and add 1 to it:
err = generator.setUint56(value + 1)
This works if partition is the 1st byte, but won't work if partition is 3rd byte.
See this case: if now the UUID for partition 2 is: 3298534883327 (hex: 00 00 02 FF FF FF FF FF)
C7 C6 P C5 C4 C3 C2 C1
00 00 02 FF FF FF FF FF
Then calling GenerateUUID() again, will call setUint56(value + 1), which becomes 00 00 03 00 00 00 00 00, which would not increment C6, and causing the counter to go back to 0 again.
We need to have a test case, for a UUID generator for partition 2 and it's value is 00 00 02 FF FF FF FF FF, the next two UUIDs should be 00 01 02 00 00 00 00 00, and 00 01 02 00 00 00 00 01.
There was a problem hiding this comment.
What happens is that we first call generator.getUint64() which gets the stored counter for that partition (00 00 00 FF FF FF FF FF). then stores it back as incremented by 1 generator.setUint56(value + 1) ( 00 00 01 00 00 00 00 00). Only after this is the value cut up and the partition bytes inserted.
value: 00 00 00 FF FF FF FF FF
partition: 02
(value & 0xFF_FF00_0000_0000) << 8) -> 00 00 00 00 00 00 00 00
(partition << 40) -> 00 00 02 00 00 00 00 00
(value & 0xFF_FFFF_FFFF) -> 00 00 00 FF FF FF FF FF
00 00 02 FF FF FF FF FF
and the next one:
value: 00 00 01 00 00 00 00 00
partition: 02
(value & 0xFF_FF00_0000_0000) << 8) -> 00 01 00 00 00 00 00 00
(partition << 40) -> 00 00 02 00 00 00 00 00
(value & 0xFF_FFFF_FFFF) -> 00 00 00 00 00 00 00 00
00 01 02 00 00 00 00 00
I do agree that a test for this continuation would be great!
There was a problem hiding this comment.
@zhangchiqing
actually problem is naming of the function, I didn't want to touch that. Actually getuint64 is just returning the counter ( without partition byte )
There was a problem hiding this comment.
We need to have a test case, for a UUID generator for partition 2 and it's value is 00 00 02 FF FF FF FF FF, the next two UUIDs should be 00 01 02 00 00 00 00 00, and 00 01 02 00 00 00 00 01.
@bluesign could you at least add this test case? Just want to make sure we have test case covering it.
There was a problem hiding this comment.
sure, of course, I missed that part of the comment
There was a problem hiding this comment.
Oh, I know where I got confused. I thought the stored register value contains the partition and the counter, but actually it is just the counter. The register ID contains the partition already.
There was a problem hiding this comment.
yeah naming of the functions are confusing, setUint56 and getUint64 makes you feel like you are setting 56 bits of 64 bit int, and then getting it as 64 bit with partition.
Co-authored-by: Janez Podhostnik <67895329+janezpodhostnik@users.noreply.github.com>
Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #4914 +/- ##
==========================================
- Coverage 55.61% 55.60% -0.01%
==========================================
Files 1002 1002
Lines 96600 96600
==========================================
- Hits 53724 53716 -8
- Misses 38820 38826 +6
- Partials 4056 4058 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>
Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>
…low-go into uuid_partition_change
|
We forgot about this one. |
Changing partition of uuid to 3rd byte for compatibility with database types and javascript MAX_SAFE_INTEGER